Fix automatic vault login - #21126
Merged
Merged
Conversation
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideMoves a pytest hook that manipulates the vault/Foreman version source from the global conftest to the Foreman-specific test conftest, so that automatic vault login/version handling only applies to relevant Foreman tests. Sequence diagram for pytest runtest protocol with Foreman foremanctl markersequenceDiagram
participant Pytest as PytestRunner
participant ForemanConftest as tests_foreman_conftest
participant AutoVault as pytest_plugins_auto_vault
participant Settings as settings
participant Test as ForemanTest
Pytest->>ForemanConftest: discover pytest_runtest_protocol hook
Pytest->>AutoVault: initialize auto_vault plugin
Pytest->>Test: start test execution
Pytest->>ForemanConftest: pytest_runtest_protocol(item, nextitem)
alt item has foremanctl marker
ForemanConftest->>Settings: set(server.version.source, upstream)
ForemanConftest->>Pytest: yield to next hook
Pytest->>AutoVault: perform auto vault login using upstream
AutoVault-->>Pytest: login complete
Pytest-->>ForemanConftest: resume hook
ForemanConftest->>Settings: set(server.version.source, internal)
else item has no foremanctl marker
ForemanConftest->>Pytest: yield to next hook
Pytest->>AutoVault: perform normal behavior
AutoVault-->>Pytest: complete
Pytest-->>ForemanConftest: resume hook
end
Pytest->>Test: run test body
Test-->>Pytest: result
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
pytest_runtest_protocol, rename the unusednextitemparameter to_nextitem(or remove it if not required by the hook signature) to make it clear it’s intentionally unused and avoid linter warnings.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `pytest_runtest_protocol`, rename the unused `nextitem` parameter to `_nextitem` (or remove it if not required by the hook signature) to make it clear it’s intentionally unused and avoid linter warnings.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
Author
|
trigger: test-robottelo |
Collaborator
|
PRT Result |
ogajduse
approved these changes
Mar 26, 2026
ogajduse
left a comment
Member
There was a problem hiding this comment.
ACK. I want @Gauravtalreja1 to take a look as well.
rmynar
approved these changes
Mar 27, 2026
Gauravtalreja1
approved these changes
Mar 30, 2026
Gauravtalreja1
left a comment
Member
There was a problem hiding this comment.
ACK, thanks @lpramuk for fixing it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem Statement
Automatic vault login when running tests or collecting tests is now broken
Solution
Move recent
conftest.pychanges (#20339) to appropriate location intests/foreman/conftest.pyRelated Issues
SAT-43736
Summary by Sourcery
Enhancements: